Catch falsified memories and sync the shared gates across both skills - #17
Closed
bguidolim wants to merge 8 commits into
Closed
Catch falsified memories and sync the shared gates across both skills#17bguidolim wants to merge 8 commits into
bguidolim wants to merge 8 commits into
Conversation
- Add git triage for zero-hit symbols (shipped-then-removed vs branch-only vs never existed) and a DROP category for memories the code refutes - Require checking whether source comments or auto-loaded instructions already carry a memory before KEEP - Move edit-ordering and delegation rules to references/execution.md; make all examples language-agnostic
There was a problem hiding this comment.
Pull request overview
Adds falsified-memory detection and safer audit execution guidance.
Changes:
- Adds three-way Git history triage and false-memory DROP criteria.
- Expands staleness and redundancy checks.
- Extracts edit and link-repair mechanics into a reference.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
skills/memory-audit/SKILL.md |
Extends audit criteria and workflow guidance. |
skills/memory-audit/references/execution.md |
Adds execution and delegation safeguards. |
Suppressed comments (1)
skills/memory-audit/references/execution.md:16
- This rule is written for every DROP, but most A–H drops (duplicates, generic guidance, self-evident fixes) have no “corrected fact”; copying their content inline defeats the deduplication/removal decision. Limit inline salvage to falsified memories with a verified observation, and otherwise repoint to an existing replacement or remove the dead link.
2. **Replace a dropped memory's inbound pointer with the corrected fact inline** — don't just delete
the bullet. The referrer then carries what the code actually does, which is strictly more useful
than the dead pointer was, and it preserves the verified half of the memory being removed.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Exclude the memories dir and print changed filenames: -S counts prose, so a memory naming a fictional API was confirming it as real code history - Gate the triage on the project being a git repo, and never DROP on an unverifiable symbol - Forbid committing memory changes — a KB may live in the project repo, a separate repo, or only on disk
- Forbid pointing git at the memories dir with -C as well as cd: when the KB is a separate repo, the symbol triage would answer about that repo's history instead of the project's - Carve out DROP category B for projects with no version control, where a historical-record memory may be the only trace of the change
- Query the default branch before widening to --all: a bare --all could match both the shipped-then-removed and branch-only rows, leaving the verdict to depend on which commit was tested - Split the inbound-pointer rule by drop reason, since only a falsified memory has a corrected fact worth inlining - Rename via copy-repoint-delete so no step leaves a dangling link; rank instruction carriers by whether they actually fire on the work
- Drop the instruction telling the skill to append new lessons to its reference file: installed skill files are hash-verified, so a self-edit reads as content drift and is lost on the next sync - State that the audit's only writes are the memory files approved in Step 4, and route session knowledge through continuous-learning
- Add a fourth SYNC block for the forcing-function test, resolving the two skills' disagreement on when a lint rule or version control actually displaces a memory - Gate capture on symbol existence so an API that never shipped cannot be recorded in the present tense - Neutralize the remaining stack-specific examples in the capture skill and drop three restated sections
- Step 2 mandated a duplicate search but printed nothing, so a skipped search was indistinguishable from one that ran — unlike the Step 4 checks, which must show their work - One logged line now records the query, the hits, and which branch was taken
- Six DROP categories, plus naming and staleness, exist on both sides unsynced; two had already drifted before this PR - A locked block is the wrong tool: audit needs exception clauses capture has no use for, and the optional audit skill can never be referenced by the required capture skill
bguidolim
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Suppressed comments (2)
skills/continuous-learning/SKILL.md:196
- An unscoped grep can resolve a fictional API from an existing memory, planning document, or other prose—the exact false-positive this change is meant to prevent. Require an implementation/configuration hit and treat prose-only matches as unresolved.
**Check 3: Symbol existence.**
Grep every symbol, path, API, or config key the draft names in the present tense. A draft asserting that "hiding is handled by `setFooHidden()`" must be able to point at `setFooHidden()` in the code. Where a name does not resolve, either cut the claim or mark it explicitly as proposed and not yet implemented — never record an intended design in the present tense. This is the cheapest place to stop a memory that prescribes an API which never shipped; once saved, only a later audit will catch it.
skills/memory-audit/SKILL.md:236
- This first grep still searches from the project root without excluding the memory files. Because the audited memory itself contains the symbol, a fabricated API can produce a nonzero hit and never enter the new C.4 zero-hit triage. Exclude the KB and require source/config hits here; prose-only matches must count as zero.
- Grep for every distinct symbol/type referenced in the batch — confirm presence, note renames or deletions. Where a symbol returns zero hits, resolve it through the C.4 triage before assigning any verdict; an empty grep is not a finding on its own.
| #### Mandatory pre-`Write` checks | ||
|
|
||
| Run both checks as visible output before any `Write` to `<project>/.claude/memories/`. Hidden reasoning is easy to skip; printed output is reviewable. | ||
| Run all three checks as visible output before any `Write` to `<project>/.claude/memories/`. Hidden reasoning is easy to skip; printed output is reviewable. |
| | Finding | Verdict | | ||
| |---|---| | ||
| | Step 1 returns commits touching **source** → it shipped, then was removed | **DROP** (cat D) — the code is gone; the memory is a historical record | | ||
| | Step 1 empty, step 2 returns commits touching source → the code is on an unmerged branch | **UPDATE**, not DROP. Add a status header naming the branch and stating the default branch's *current* values, so the memory is useful either way and self-corrects when the branch merges | |
Comment on lines
+13
to
+17
| `grep -rl '<memory-name>' <memories-dir>`. To rename: copy to the new name, repoint every referrer, | ||
| *then* delete the old file. In that order the KB is consistent after each step, so an interrupted | ||
| audit leaves a duplicate at worst, never a dangling pointer. `mv` cannot give you that — it breaks | ||
| inbound links the moment it runs, while repointing first writes links to a name that does not exist | ||
| yet. To delete: repair the referrers before removing the file. |
Comment on lines
+283
to
286
| Read [references/execution.md](references/execution.md) before the first batch's edits land. It covers link-repair ordering, what to verify in the parts you KEEP, and the rules for delegating batches to subagents — each one a way a real audit has damaged the KB it was cleaning. | ||
|
|
||
| - **DROP**: Delete the file with `Bash(rm <path>)` | ||
| - **UPDATE (rename)**: Rename with `Bash(mv <old> <new>)` |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The audit could tell a stale memory from a fresh one, but not from a false one — a memory confidently describing an API that never shipped, or asserting a prohibition the code now contradicts. A session that trusts one deletes working code or hunts a phantom. The audit now flags that class explicitly, and the capture side refuses to record a symbol it cannot find in the code, so the defect stops where it is created instead of surviving until someone runs an audit.
A zero-hit search was also being read as one finding when it is three: code that shipped and was removed, code sitting on an unmerged branch, and code that never existed. Collapsing those either throws away verified reasoning or preserves fiction. The triage now separates them deterministically, and it no longer assumes what it used to: that the project is a git repo, that a knowledge base lives in git at all, or that a name found in tracked text is evidence of code. That last one mattered most — memories are committed to the project repo unless someone opts out, so a memory naming a fictional API was confirming it as real history and feeding a fabricated rationale into the approval gate the audit depends on.
Underneath both, the two skills were stating shared rules twice and had already drifted apart: they disagreed on whether a lint warning counts as enforcement, and only one of them knew that a project without version control keeps its history nowhere else. The shared test is now a single locked block checked in CI. The sections that genuinely cannot be locked are documented instead, with the reason.
The audit also no longer writes anywhere except the memory files a user approved for the current batch — not its own skill files, and never git.
Test plan
git fetch --allbefore any "never existed" verdict is trusted. In a project that is not a git repo, expect the triage skipped and the symbol reported unverifiable rather than dropped.